fix(highlight): keep long tokens colored after the regexp2 2.7.1 bump - #330
Merged
Conversation
…eir color regexp2 v2.7.1 added a default MaxBacktrackingStackSize of 100000 that did not exist in v2.2.1. Chroma compiles every lexer rule with a bare regexp2.Compile and its matchRules drops results whose match returned an error, so a rule that trips the cap degrades to a non-match with nothing logged. A Go string literal over roughly 17k characters was repainted as chroma.Error, and revdiff could not see it because highlightFile only checks the error Tokenise itself returns. Raise the package default from New(), which is the only route into chroma's deferred rule compilation. The cap stays finite rather than going back to unbounded: revdiff opens arbitrary repos and 2.7.1 added the bound for a reason. Multi-megabyte tokens still exceed it, and the comment says so. regexp2 moves from indirect to direct in go.mod as a result.
re-measured during the PR #327 review: the app package now runs 81.5s and 87.5s on master against 68s before, so headroom against the 100s budget is down from roughly 32s to roughly 13s. Still later rather than yes, but the next addition to the launcher matrix is what flips it.
10m covered roughly 1.7M characters, far past anything revdiff needs, and the cost was not free. growTrack doubles until it reaches the cap, so the steps before ErrBacktrackingStackLimit scale with it, while chroma sets a separate 250ms MatchTimeout on every rule and discards that error through the same err == nil gate. A cap that large turns a sub-millisecond abort into a stall of up to a quarter second on the bubbletea event loop, which themeselect hits on every keypress that changes the chroma style. The chroma Go quoted-string rule needs 240045 slots for the 40k test input, so 1m keeps about four times headroom and still fixes the regression. Comment corrections that go with it: slots are ints, so 1m is about 8MB on 64-bit; the cap bounds runtrack only, since runstack grows through doubleIntSlice with no limit check, so the two together reach roughly twice the nominal budget; and the 250ms timeout is a second independent ceiling that raising this constant cannot help. New's godoc now names the process-wide change to regexp2.DefaultOptimizationOptions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The dependency refresh in #327 moved
dlclark/regexp2/v2from 2.2.1 to 2.7.1, which added a defaultMaxBacktrackingStackSizeof 100000 that did not exist before. Chroma compiles every lexer rule with a bareregexp2.Compileand itsmatchRuleskeeps only results whose match returned no error, so a rule that trips the cap degrades to a non-match with nothing logged. A Go string literal over roughly 17k characters gets repainted aschroma.Error, and revdiff cannot detect it becausehighlightFileonly checks the errorTokeniseitself returns.Reachable on files people actually review and modify: an SVG with a long
<path d="...">, a base64 data URI in CSS, a source map'smappingsstring, a fixture with an embedded PEM.The fix. Raise regexp2's package default from
New(), which is the only route into chroma's deferred rule compilation. The cap stays finite rather than going back to unbounded, since revdiff opens arbitrary repos and 2.7.1 added the bound deliberately.Why 1m and not higher.
growTrackdoubles until it reaches the cap, so the number of backtracking steps beforeErrBacktrackingStackLimitscales directly with it. Chroma also sets a separate 250msMatchTimeouton every rule and discards that error through the sameerr == nilgate, so a large cap does not remove the failure, it just makes the timeout the binding ceiling and converts a sub-millisecond abort into a stall of up to a quarter second. That stall falls on the bubbletea event loop, whichthemeselecthits on every keypress that changes the chroma style. The chroma Go quoted-string rule needs 240045 slots for the 40k test input, so 1m keeps about four times headroom at roughly 8MB rather than 76MB.regexp2moves from indirect to direct in go.mod as a result.The const comment records the measured threshold, the slot-to-bytes conversion, the fact that the cap bounds
runtrackonly whilerunstackgrows throughdoubleIntSlicewith no limit check, and the 250ms timeout as a second independent ceiling.New's godoc names the process-wide change toregexp2.DefaultOptimizationOptions.The regression test fails at the old 100000 default and passes at 1m.
One unrelated commit rides along: a refresh of the
make racetimeout backlog note, re-measured while reviewing #327.Related to #327